Repository navigation
fix(agc): fault image table entries the device or guest memory cannot hold - #6481
fdfawef324 wants to merge 9 commits into
Conversation
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
P2 source-review finding at The Vulkan query contract defines that result as an unsupported combination. Broad dimension/layer limits cannot establish support for it; see also image-creation support requirements. Please turn the explicit unsupported result into an ordinary descriptive entry refusal before creation. Suggested regression in the existing recording mock: return Source/specification review only. A native regression patch was prepared and checked for applicability, but remains uncompiled/unexecuted. No GPU failure or connection to the observed title device loss is established. |
…ported RequireSampledImageWithinDeviceLimits fell back to VkPhysicalDeviceLimits when vkGetPhysicalDeviceImageFormatProperties reported VK_ERROR_FORMAT_NOT_SUPPORTED for the format, type, usage and create flags of the entry's sampled image. A small entry in registered memory then passed and the texture lookup called vkCreateImage with that combination, which the device reports unsupported (VUID-VkImageCreateInfo-imageCreateMaxMipLevels-02251). Such an entry is now refused before its guest memory is read, naming the format, type and flags, and faults when selected like the other refused entries; the device-wide limits are no longer consulted. Other query failures still propagate as VulkanError. Test: agc_driver_graphics (tests/ImageTableEntryValidation.cpp) resolves a registered 8x8 RGBA8 entry while the mock's format query reports VK_ERROR_FORMAT_NOT_SUPPORTED and checks that the query ran, the entry faults and no image is created. Without this change it fails with "an 8x8 registered texture whose exact image combination is unsupported: the entry was served instead of faulting when selected; images created: 8x8x1/1 layers". Reported in review of boykopovar#6481. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Confirmed at e441ee7: Fixed in c578222: an unsupported combination is now refused before the entry's guest memory is read (the message names the VkFormat, image type and create flags), through the existing Your regression is in AI-assisted: yes |
46f6392 to
ce99615
Compare
|
ds-main5-c01: one image table entry's out-of-memory error stopped Demon's Souls (PPSA01342) Windows Server 2025, RTX 3060 12 GB, driver 591.86, 32 GB RAM. The build was
What changed on the branch (force-pushed,
This is one run, and it does not show whether a shader selects the entry. AI-assisted: yes |
|
Tested on Linux (Arch), This PR and #6402 break Cause: this PR registers Whichever PR lands second should drop its duplicate registration. AI-assisted: yes |
…ported RequireSampledImageWithinDeviceLimits fell back to VkPhysicalDeviceLimits when vkGetPhysicalDeviceImageFormatProperties reported VK_ERROR_FORMAT_NOT_SUPPORTED for the format, type, usage and create flags of the entry's sampled image. A small entry in registered memory then passed and the texture lookup called vkCreateImage with that combination, which the device reports unsupported (VUID-VkImageCreateInfo-imageCreateMaxMipLevels-02251). Such an entry is now refused before its guest memory is read, naming the format, type and flags, and faults when selected like the other refused entries; the device-wide limits are no longer consulted. Other query failures still propagate as VulkanError. Test: agc_driver_graphics (tests/ImageTableEntryValidation.cpp) resolves a registered 8x8 RGBA8 entry while the mock's format query reports VK_ERROR_FORMAT_NOT_SUPPORTED and checks that the query ran, the entry faults and no image is created. Without this change it fails with "an 8x8 registered texture whose exact image combination is unsupported: the entry was served instead of faulting when selected; images created: 8x8x1/1 layers". Reported in review of boykopovar#6481. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ce99615 to
d1f7857
Compare
|
Thank you, @nicolasfalesy. Your reading is right. Our agc_driver_image_table change registered FloatTexels, UintTexels, SintTexels and DepthStorage once in main() for the whole run. #6402's RunStoreTests registers the same three texel arrays again, and the guest allocation registry throws "overlapping guest allocation" when an address is registered twice. Each PR passes alone, and the two fail together. We confirmed that from the code; we have not reproduced your lavapipe run. What changed (new head d1f7857, was ce99615; rebased onto current main c842491):
Checked on CPU, without a GPU:
GPU, on the RTX 3060 (driver 591.86, Windows Server 2025): on this head The test change is folded into the first commit of this PR, so the push is a force-push; the difference to the previous head is AI-assisted: yes |
… hold A bindless image table is resolved whole: every entry's T# becomes a Vulkan image before the shader runs, whether or not it is ever selected. An entry was refused only when its surface had no committed host page, so garbage records reached vkCreateImage. In a Demon's Souls (PPSA01342) run a 1547-entry table held a T# at 0x200 of 256x256 R16G16B16A16_FLOAT with 7425 array layers (3.6 GiB). The host pages below 3.6 GiB passed the mapping check, the driver snapshotted host process memory into a texture and created an image of 7425 layers on an RTX 3060, whose limit is 2048 (VUID-VkImageCreateInfo-arrayLayers-02256), and the device was lost during that table's resolution. Each table entry is now checked before its guest memory is read or any image is created: - RequireSampledImageWithinDeviceLimits compares the image the sampled upload would create (format, type, cube and mutable-format flags, extent, layers) with vkGetPhysicalDeviceImageFormatProperties for it (cached per physical device, format, type and flags), or with VkPhysicalDeviceLimits when the query reports VK_ERROR_FORMAT_NOT_SUPPORTED. Level 0 of the image is a lower bound of its size, compared with maxResourceSize. The mip count is not checked here. - The surface must overlap a readable registered guest allocation (RegisteredReadableOverlaps, the registry RegisteredReadableCovers already reads for storage write-backs) instead of any committed host page. A surface that overlaps one is read as before. A refused entry is handled like the entries the driver already rejects: it faults when the shader selects it (the dispatch reports "rejected by the driver" with the reason) and has no effect otherwise. The console samples such a T#; the technical debt records it. Graphics::Check throws VulkanError (a std::runtime_error with the VkResult), and resolveImageTables rethrows it from the entry checks and the texture lookup instead of turning it into a faulting entry, so a device loss or an allocation failure while an entry's texture is made reaches the draw or dispatch. Test: agc_driver_graphics (tests/ImageTableEntryValidation.cpp) resolves tables against a recording mock device with maxImageArrayLayers 2048. The T# at 0x200 encoded from the logged fields, an 8x8 texture in unregistered host memory, a 2049-layer array and a 16384x1 texture over a format extent of 8192 fault without vkCreateImage; an 8x8 texture and a 2048-layer array in registered memory next to them are created. A 64x64 RGBA8 texture faults under a maxResourceSize of 8192 while a 32x32 one is created, a texture whose second page alone is registered is created, and VK_ERROR_DEVICE_LOST from an entry's vkCreateImage reaches the caller. With the ShaderResources.cpp change reverted it fails with "the 256x256 2D array of 7425 layers at 0x200: the entry was served instead of faulting when selected; images created: 8x8x1/1 layers 256x256x1/7425 layers 8x8x1/1 layers 8x8x1/2049 layers 8x8x1/2048 layers 16384x1x1/1 layers". agc_driver_image_table, agc_driver_bindless_images and agc_driver_image_sint8 register the static texels their tables sample. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ported RequireSampledImageWithinDeviceLimits fell back to VkPhysicalDeviceLimits when vkGetPhysicalDeviceImageFormatProperties reported VK_ERROR_FORMAT_NOT_SUPPORTED for the format, type, usage and create flags of the entry's sampled image. A small entry in registered memory then passed and the texture lookup called vkCreateImage with that combination, which the device reports unsupported (VUID-VkImageCreateInfo-imageCreateMaxMipLevels-02251). Such an entry is now refused before its guest memory is read, naming the format, type and flags, and faults when selected like the other refused entries; the device-wide limits are no longer consulted. Other query failures still propagate as VulkanError. Test: agc_driver_graphics (tests/ImageTableEntryValidation.cpp) resolves a registered 8x8 RGBA8 entry while the mock's format query reports VK_ERROR_FORMAT_NOT_SUPPORTED and checks that the query ran, the entry faults and no image is created. Without this change it fails with "an 8x8 registered texture whose exact image combination is unsupported: the entry was served instead of faulting when selected; images created: 8x8x1/1 layers". Reported in review of boykopovar#6481. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The map and image bindings of tests/ImageTableEntryValidation.cpp left DescriptorBinding::descriptorSet, which has no default member initializer, uninitialized. ShaderResources refuses a binding outside its stage's descriptor set, so the test depended on stack contents: on a build where the garbage was not 0 the whole agc_driver_graphics run stopped with "unexpected descriptor set: a shader resource must use its stage's descriptor set". Both bindings are value-initialized and use RuntimeAbi::DescriptorSet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An image table entry within the format's array layers, extent and maxResourceSize was snapshotted and given an image even when level 0 of the image alone needed more bytes than any memory heap holds. No memory type can back such an image: an allocation larger than its heap is invalid usage (VUID-vkAllocateMemory-pAllocateInfo-01713). A garbage T# reaches this easily: in a Demon's Souls (PPSA01342) run on an RTX 3060 (driver 591.86) one entry whose first four dwords describe a 1024x1024 render target decoded with a last slice of 1792 from its other dwords, 1793 layers and 7520387072 bytes, within the device's 2048 layers. That image fits the 16334.9 MiB host heap; the same last slice over a 2048x2048 surface would fit no heap. RequireSampledImageWithinDeviceLimits now also compares level 0 of the image, the lower bound its maxResourceSize check uses, with the largest heap a memory type allocates from, and the entry faults when selected before its guest memory is read. The bound is the heap's size, not what its VK_EXT_memory_budget budget leaves: in that run the budget showed no host heap with room for the 7520387072-byte image (stderr L11044) and the driver then allocated it in host heap 1 (L11045), so the budget is not a limit the device enforces. The debt file records the bound. Test: agc_driver_graphics (tests/ImageTableEntryValidation.cpp). A 256x256 RGBA8 array of 3 layers (786432 bytes) is created in heaps of 4 GiB whose budgets leave 524288 and 262144 bytes; in heaps of 524288 and 262144 bytes an array of 2 layers is created and one of 3 layers faults with no image and no upload buffer. Without the Texture.cpp change it fails with "a 256x256 RGBA8 array of 3 layers (786432 bytes) in heaps of 524288 and 262144 bytes: the entry was served instead of faulting when selected; images created: 256x256x1/2 layers 256x256x1/3 layers". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
adriadam10
left a comment
There was a problem hiding this comment.
This conflicts with main in ShaderResources.cpp, Texture.cpp, tests/execution/ImageTable.cpp and docs/dev/debt/libSceAgcDriver.md, so the Build run can't test it as it would land. Please rebase.
The Texture.cpp conflict is not only textual: main now allocates the image through AllocateImageMemory(context, allocation, requirements.memoryTypeBits, "vkAllocateMemory texture"), which already retries in host memory when a device-local allocation is refused. Please re-derive what ownTextureMemory / TextureMemoryExhausted should wrap on top of it:
- which
VK_ERROR_OUT_OF_*results can still reach the entry afterAllocateImageMemoryhas fallen back (and for thevkCreateImage, staging, tiled and linear buffer paths that still allocate directly); - the second-run case in the description (7520387072-byte allocation,
initialResult=-2in host heap 1) should be re-checked against that fallback, and theImageTableEntryValidationOOM cases updated so they still fail without the change.
The cachedTexture call in resolveImageTables also gained a multisampleViewType argument on main; keep it when merging the new catch clauses.
d1f7857 to
1be24da
Compare
|
@adriadam10 rebased to The snapshot retains main's complete The historical 7.52 GB upload refusal was already in host heap 1 after reclaim; device-to-host fallback cannot provide more room there. I have qualified the body: that old run is not a test of this head or proof that the entry remains unselected. The format preflight's sRGB usage/expansion limits are also explicit. The allocation mock now tracks the same image across retries. One temporary refusal must recover; persistent refusals fault the entry and permit the next one. Removing only the common allocator or only the ordinary sRGB classifier produces the intended new assertions. Both CPU/mock suites pass; the five required-Vulkan image-table adjacency cases pass on RTX 3060 in 5.22 s without skips. Conventions and a clean merge-tree against GPUs tested: NVIDIA GeForce RTX 3060 12 GB, driver 591.86 (32.0.15.9186), Vulkan 1.4.325, Windows Server 2025 10.0.26100. AI-assisted: yes (Claude Code and Codex) Co-Authored-By: Codex noreply@openai.com |
adriadam10
left a comment
There was a problem hiding this comment.
The queue's full Build on 1be24da14 fails linux:agc_draw_descriptors, which main does not fail:
AGC graphics: the draw snapshot wrote the wrong number of image table descriptors into the vertex set
Cause: ShaderResources.cpp:4749 now requires RegisteredReadableOverlaps(...) where main accepted any accessible memory. drawSnapshotTableSetTests (tests/Recorder.cpp, the texels array around line 3173) points its table entries at an unregistered static std::array, so every entry now faults when resolved and the expected 1 (vertex) / 2 (fragment) descriptor writes don't happen.
Please register that array as a guest allocation in the test (like the image-table tests do), and run agc_draw_descriptors plus the other tests that build image-table entries from host memory (grep -rn "imageTable\|ImageTable" core/libs/prx/libSceAgcDriver/tests/*.cpp), since the stricter check can affect any of them. Once the full Build is green this is ready to merge.
|
One coverage distinction for coordination: |
|
stale+rebase 1. Freshness Envelope
2. Findings
3. Cross-PR Ledger
|
|
@fdfawef324 a title now needs this check on current
This PR has conflicts and has been 11,944 commits behind since 10-30. Do you plan to rebase it? If not, I can open the narrow check on its own, with a test that fails on AI-assisted: yes (Claude Code) |
What
Resolving a bindless image table creates textures before the shader selects an entry. Descriptors over the sampled-image or heap limits, and surfaces in unregistered host memory, could therefore reach image creation even when the entry was never used.
Validate registered guest-memory overlap, the ordinary sampled image's format combination, extent, layers and minimum footprint before reading its surface. Refused entries fault when selected. A final host/device memory refusal from an entry's own snapshot image or upload allocation also becomes an entry fault; that incomplete table is resolved again on its next use. Other errors from the sampled-texture lookup, including device loss and submission failure, propagate.
The rebase preserves main's
multisampleViewTypeargument, selected-column range handling and scoped test registrations. The ordinary snapshot still callsAllocateImageMemory: pool trim, texture reclaim, retry and allowed host fallback run before an eventual allocation refusal is classified.ownTextureMemorywraps that complete operation, rather than replacing it with raw allocation. It also wraps the snapshot's own image creation/binding, upload allocations and ordinary sRGB expansion. Storage-copy expansion and recorder submission failures retain their existing propagation.The historical 7,520,387,072-byte Demon's Souls allocation was already attempting host heap 1 when its upload allocation failed after reclaim (
initialResult=-2, two attempts). Main's device-to-host fallback does not create additional host-heap capacity. This explains why a final refusal can still reach the entry classifier; the historical run is not a new execution on this head, and does not show whether that entry would be selected. No title memory, FPS or first-level improvement is claimed.Limits recorded in technical debt: format preflight queries the original sampled format with
TRANSFER_DST | SAMPLED. It does not separately preflight the sRGB upload's additionalTRANSFER_SRCusage or its secondary mutable RGBA8 expansion combination. The written-entry storage lookup retains its inheritedstd::runtime_errorcatch. Selecting a driver-refused entry remains a documented emulation limitation.Tested
Exact clean candidate
1be24da14e8a0e9afecfa4f558018ce19d1bd895, rebased on84c1a4d9a26429cb483ec963195cff8ef25d128b; Windows Server 2025, MinGW GCC 15.2 Release/O3. Seven affected targets build. CPU/mockagc_driver_graphicsandagc_driver_image_table_cachepass, 2/2 in 10.84 seconds. The selected entry-validation suite also passes, with its arena/write-watch checks exercised.The recording mock keeps image identity across allocation retries. A temporary first refusal must recover and serve the entry. Persistent image/upload refusals must fault only that entry, release its resources, allow the next entry, and retry resolution on later use. Device loss and submission failure retain typed error propagation. The ordinary sampled sRGB expansion has separate allocation-refusal and device-loss controls.
Two selective CPU regression controls fail at their intended assertions: removing only the sRGB expansion classifier fails
the expanded sampled image classified its allocation failure incorrectly; replacing only the normal snapshot's common allocator with raw allocation failsthe sampled image did not retry its initial device-memory refusal. Build plus selected-test feedback was 46.08 and 41.40 seconds respectively. Final source was restored and rebuilt clean. These controls test allocation contracts with mocks; they do not measure title performance.Fresh managed-slot Vulkan checks on the exact candidate:
agc_driver_bindless_images,agc_driver_image_sint8,agc_driver_image_table,agc_driver_image_table_cache_boundsandagc_driver_image_table_reuse, 5/5 pass in 5.22 seconds, no skipped cases or reported skipped subcases, with Vulkan required and shader caching disabled. Actual binaries/libraries were hash-checked before and after. Independent source review and 308 readiness pins were checked. Generated TraceRevision is1be24da14e8a, dirty 0; natural shader SourceVersion is0xb9fb57da52370e78.Conventions pass against fetched main
676d423968a1a8fcc79c6247020ce22090a8e0d4; a clean merge-tree into that main was checked separately. All execution results above belong to the exact candidate, not the merge tree. Earlier RTX 3060 results and @nicolasfalesy's Linux/lavapipe composition report remain evidence for their recorded older heads. No new Linux, full-suite, title or real exhausted-heap result is claimed.Checklist
main; no other open PR implements these entry checks (nearby image-table reuse/registration work is preserved).mdfiles or images added to the repositoryCo-Authored-By: Claude Opus 5.5 noreply@anthropic.com
Co-Authored-By: Codex noreply@openai.com